Add ASAN and Valgrind integration for CI test suite by fdcastel · Pull Request #289 · FirebirdSQL/firebird-odbc-driver

fdcastel · 2026-04-10T23:27:58Z

Summary

Integrate AddressSanitizer (ASAN) and Valgrind into the CI test pipeline so that memory bugs (buffer overflows, use-after-free, leaks, etc.) are caught automatically on every PR.

Closes #288

Changes

CMake (`CMakeLists.txt`)

ENABLE_ASAN option (default OFF, Linux/GCC/Clang only): appends -fsanitize=address -fno-omit-frame-pointer to compile and link flags. Suppresses -DLOGGING in Debug builds for cleaner sanitizer output.
ENABLE_VALGRIND option (default OFF): finds the valgrind binary and configures MEMORYCHECK_COMMAND / MEMORYCHECK_COMMAND_OPTIONS for ctest -T memcheck.
Mutual exclusion enforced: enabling both emits a FATAL_ERROR.
Both options are guarded behind if(NOT MSVC) since MSVC ASAN has different semantics (future work).

CMake Presets (`CMakePresets.json`)

asan configure/build/test presets inheriting from debug, with ASAN_OPTIONS environment variable.
valgrind configure/build/test presets inheriting from debug, with a 600s test timeout (Valgrind is ~10-20x slower).

Build script (`firebird-odbc-driver.build.ps1`)

New -Sanitizer parameter accepting None, Asan, Valgrind.
Passes the corresponding -DENABLE_ASAN=ON or -DENABLE_VALGRIND=ON to CMake.
Sets ASAN_OPTIONS / LSAN_OPTIONS environment variables at runtime for ASAN builds.
Emits a warning and falls back to normal build on Windows.

CI (`build-and-test.yml`)

New Linux x64 ASAN matrix entry (ubuntu-22.04, Debug config).
New Linux x64 Valgrind matrix entry (ubuntu-22.04, Debug config) with valgrind package installation.
Sanitizer jobs set ASAN_OPTIONS and LSAN_OPTIONS environment variables.
Sanitizer jobs do not upload release artifacts.

Valgrind suppressions (`valgrind.supp`)

Initial suppressions file seeded with a rule for libfbclient.so global initialization leaks.
To be populated as false positives are discovered.

Local usage

# ASAN build
cmake --preset asan
cmake --build --preset asan
ctest --preset asan

# Valgrind build
cmake --preset valgrind
cmake --build --preset valgrind
ctest --preset valgrind

On Linux, sizeof(wchar_t) = 4 bytes but sizeof(SQLWCHAR) = 2 bytes (unixODBC defines SQLWCHAR as unsigned short). The ConvertingString constructor used sizeof(wchar_t) to convert a byte-count argument into the number of narrow characters needed: lengthString = length / sizeof(wchar_t); // = 12/4 = 3 on Linux For SQLGetDiagRecW the state buffer is declared as State(12, sqlState), giving lengthString=3 and Alloc() allocating 3+2=5 bytes. strcpy() then writes the 5-character SQL state plus its NUL terminator (6 bytes) into that 5-byte buffer, producing a 1-byte heap-buffer-overflow caught by AddressSanitizer. The same latent bug exists in SQLErrorW (same State(12, sqlState) pattern). Fix: divide by sizeof(SQLWCHAR) instead of sizeof(wchar_t). sizeof(SQLWCHAR) == 2 on all platforms (Windows: SQLWCHAR=wchar_t=2; Linux/unixODBC: SQLWCHAR=unsigned short=2), so the formula now yields: lengthString = 12 / sizeof(SQLWCHAR) = 6 and Alloc() allocates 6+2=8 bytes, comfortably holding the SQL state. On Windows sizeof(wchar_t)==sizeof(SQLWCHAR)==2, so this change is a no-op there. Found by: AddressSanitizer (introduced in CI via PR FirebirdSQL#288/FirebirdSQL#289) Test: DataTypeTest.SmallintRoundTrip -> ExecIgnoreError -> SQLExecDirect -> unixODBC dispatcher -> SQLGetDiagRecW -> sqlGetDiagRec(strcpy)

fdcastel · 2026-04-10T23:58:21Z

Yay! They are working!!!

fdcastel · 2026-04-11T00:05:44Z

Bug found and fixed by ASAN

The ASAN CI job introduced by this PR immediately caught a real memory bug in the driver.

What ASAN reported

The first ASAN run failed on DataTypeTest.SmallintRoundTrip with:

AddressSanitizer: heap-buffer-overflow
WRITE of size 6 at ... (0 bytes past end of 5-byte region)
  #0 __interceptor_strcpy
  #1 OdbcError::sqlGetDiagRec
  #2 SQLGetDiagRecW

The full output is in the CI job log: GitHub Actions -> Build and Test -> job ubuntu-22.04 / Debug / Asan -> step Run build script.

Root cause

SQLGetDiagRecW allocates a narrow-char work buffer for the SQL state:

ConvertingString<> State( 12, sqlState );

In ConvertingString's constructor, the byte-count to char-count conversion was:

lengthString = length / sizeof(wchar_t);   // BUG

On Windows sizeof(wchar_t) == sizeof(SQLWCHAR) == 2, so 12/2 = 6 -> 8-byte buffer, no overflow.
On Linux sizeof(wchar_t) == 4 but sizeof(SQLWCHAR) == 2 (unixODBC defines SQLWCHAR as unsigned short), so 12/4 = 3 -> 5-byte buffer.

OdbcError::sqlGetDiagRec then copies the SQL state ("HY000\0", 6 bytes) into that 5-byte buffer via strcpy -> 1-byte heap overflow. The same latent bug exists in SQLErrorW (same State(12, sqlState) pattern).

The bug was silently harmless on Windows and went undetected for years.

Fix

One-line change in MainUnicode.cpp (commit 636972b):

// Before
lengthString = length / sizeof(wchar_t);
// After
lengthString = length / sizeof(SQLWCHAR);

sizeof(SQLWCHAR) == 2 on all platforms, giving a correct 8-byte buffer on Linux.

CI after fix

All 4 matrix jobs now pass with no continue-on-error guard:

Job	Result
ubuntu-22.04 / Release	pass
ubuntu-22.04 / Debug / ASAN	pass (375/375 tests)
ubuntu-22.04 / Debug / Valgrind	pass
windows-latest / Release	pass

This is a concrete demonstration that ASAN integration pays off immediately.

fdcastel · 2026-04-11T00:10:09Z

@irodushka Enjoy! 😉

irodushka · 2026-04-11T18:57:48Z

Hi @fdcastel

Can you please resolve the conflicts after merging PR#286
I'll look at this PR after Orthodox Easter)

Cheers!

fdcastel · 2026-04-11T20:05:37Z

Can you please resolve the conflicts after merging PR#286 I'll look at this PR after Orthodox Easter)

Sure! I’ll look into this in a few hours.

Add CMake options ENABLE_ASAN and ENABLE_VALGRIND (Linux/GCC/Clang only, mutually exclusive) with corresponding compiler/linker flags and CTest memcheck configuration. CMake changes: - ENABLE_ASAN appends -fsanitize=address -fno-omit-frame-pointer to compile and link flags; suppresses -DLOGGING in Debug builds for cleaner sanitizer output - ENABLE_VALGRIND finds the valgrind binary and configures MEMORYCHECK_COMMAND for ctest -T memcheck - Mutual exclusion enforced via FATAL_ERROR CMake presets: - Add 'asan' and 'valgrind' configure/build/test presets inheriting from 'debug', with ASAN_OPTIONS env and Valgrind timeout multiplier Build script (firebird-odbc-driver.build.ps1): - Add -Sanitizer parameter (None, Asan, Valgrind) that passes the corresponding CMake option; sets ASAN_OPTIONS at runtime; skips sanitizers on Windows with a warning CI (build-and-test.yml): - Add Linux x64 ASAN matrix entry (ubuntu-22.04, Debug) - Add Linux x64 Valgrind matrix entry (ubuntu-22.04, Debug) with valgrind package installation - Sanitizer jobs do not upload release artifacts Also adds valgrind.supp suppressions file (seeded with fbclient rule). Closes FirebirdSQL#288

LSAN_OPTIONS referenced a relative 'lsan.supp' path but CTest runs from the build directory. Use an absolute path derived from the source/workspace root so the file is always found. Also adds the lsan.supp suppressions file with initial rules for libfbclient, libodbc, and libodbcinst.

ASAN correctly detected a pre-existing heap-buffer-overflow in OdbcError::sqlGetDiagRec (strcpy into an undersized ConvertingString buffer). This proves the sanitizer integration works, but the existing bug should not block PRs. Mark the ASAN matrix entry continue-on-error: true until the underlying memory bugs are fixed in a separate PR.

On Linux, sizeof(wchar_t) = 4 bytes but sizeof(SQLWCHAR) = 2 bytes (unixODBC defines SQLWCHAR as unsigned short). The ConvertingString constructor used sizeof(wchar_t) to convert a byte-count argument into the number of narrow characters needed: lengthString = length / sizeof(wchar_t); // = 12/4 = 3 on Linux For SQLGetDiagRecW the state buffer is declared as State(12, sqlState), giving lengthString=3 and Alloc() allocating 3+2=5 bytes. strcpy() then writes the 5-character SQL state plus its NUL terminator (6 bytes) into that 5-byte buffer, producing a 1-byte heap-buffer-overflow caught by AddressSanitizer. The same latent bug exists in SQLErrorW (same State(12, sqlState) pattern). Fix: divide by sizeof(SQLWCHAR) instead of sizeof(wchar_t). sizeof(SQLWCHAR) == 2 on all platforms (Windows: SQLWCHAR=wchar_t=2; Linux/unixODBC: SQLWCHAR=unsigned short=2), so the formula now yields: lengthString = 12 / sizeof(SQLWCHAR) = 6 and Alloc() allocates 6+2=8 bytes, comfortably holding the SQL state. On Windows sizeof(wchar_t)==sizeof(SQLWCHAR)==2, so this change is a no-op there. Found by: AddressSanitizer (introduced in CI via PR FirebirdSQL#288/FirebirdSQL#289) Test: DataTypeTest.SmallintRoundTrip -> ExecIgnoreError -> SQLExecDirect -> unixODBC dispatcher -> SQLGetDiagRecW -> sqlGetDiagRec(strcpy)

The heap-buffer-overflow in ConvertingString (sizeof(wchar_t) vs sizeof(SQLWCHAR)) has been fixed. ASAN now passes all 375 tests cleanly on ubuntu-22.04/GCC, so the soft-fail guard is no longer needed.

fdcastel · 2026-04-11T20:31:41Z

@irodushka Rebased onto current master.

Conflicts were in build-and-test.yml and firebird-odbc-driver.build.ps1, caused by the expanded matrix (Windows ARM64, Windows x86/WoW64, Linux ARM64, multi-Firebird-version runs) introduced by master.

Resolution: kept all new upstream matrix entries and added the sanitizer entries on top; merged the Architecture and Sanitizer params in the build script so both features coexist.

- Move option() declarations outside if(NOT MSVC) guard; emit warning on MSVC instead of hiding the options entirely - Rename ENABLE_ASAN -> BUILD_WITH_ASAN and ENABLE_VALGRIND -> BUILD_WITH_VALGRIND to follow existing BUILD_* naming convention - Remove duplicate target_compile_definitions for DEBUG/LOGGING (already defined via add_compile_options in Linux section) - Separate LOGGING into add_definitions() and conditionally skip it for both ASAN and Valgrind builds (not just ASAN)

Per PR review feedback: consolidate the build-type options matrix and express the sanitizer carve-out via add_definitions/remove_definitions instead of an inverted if-NOT guard. Behavior is unchanged — LOGGING is defined for Debug builds and stripped for ASAN/Valgrind builds.

fdcastel · 2026-04-15T20:17:49Z

@irodushka Thanks for the valuable insights.

I’ve just pushed all the changes. Please take a look when you have a moment.

Per review feedback: remove_definitions() cannot undo definitions added via add_compile_options(), so the add-then-remove pattern was misleading. Collapse to a single if() with the combined condition instead.

irodushka · 2026-04-16T18:34:13Z

@fdcastel

We have a bug!

-- Git describe: 52f0495
CMake Warning at cmake/GetVersionFromGit.cmake:75 (message):
  Could not parse git describe output: 52f0495
Call Stack (most recent call first):
  CMakeLists.txt:15 (include)


CMake Warning at cmake/GetVersionFromGit.cmake:76 (message):
  Using fallback version 0.0.0.0
Call Stack (most recent call first):
  CMakeLists.txt:15 (include)

The reason is in the --exclude "*-*" option -
git describe --tags --match "v[0-9]*.[0-9]*.[0-9]*" --long --always returns v3.5.0-rc1-8-g52f0495, but git describe --tags --match "v[0-9]*.[0-9]*.[0-9]*" --long --always --exclude "*-*" returns just the tail 52f0495

Can you please remind me what do you expect specifying the --exclude option here?.. And what's the purpose of the logic - first try with the --exclude, and if we got an error (GIT_DESCRIBE_RESULT==0) try without --exclude?
Obviously this logic is not working.

The first git describe call used --always, which made it return the short commit hash instead of failing when --exclude "*-*" ruled out every matching tag (as is the case today: only prerelease tags like v3.5.0-rc5 exist). The non-zero exit that was supposed to trigger the fallback without --exclude never fired, so parsing dropped to 0.0.0.0. Drop --always from both calls: if the stable-only call finds nothing it exits non-zero and the fallback runs; if the fallback also finds nothing the existing warning path reports it.

fdcastel · 2026-04-16T18:53:53Z

@irodushka you're right, good catch. Fixed in b7aa955.

Intent of the logic

--match "v[0-9]*.[0-9]*.[0-9]*" restricts candidates to semver tags, skipping legacy v1-1-release etc.
--exclude "*-*" was meant to further restrict to stable vX.Y.Z tags (excluding prereleases like v3.5.0-rc5) so that a local build would be numbered off the latest stable release.
If there is no stable vX.Y.Z tag yet (currently the case — every tag in the repo has a prerelease suffix), the second call drops --exclude and picks up the prerelease tag. The parser regex already handles the optional -<prerelease> suffix.

Why it was broken

--always on the first call tells git describe to fall back to the short commit hash when nothing matches, and in that case the command exits 0. So GIT_DESCRIBE_RESULT was 0, the if(NOT ... EQUAL 0) branch never fired, and GIT_DESCRIBE_OUTPUT ended up as just 52f0495, which then failed to match the parser regex → fallback to 0.0.0.0.

The fix

Drop --always from both calls. The first call now exits non-zero when every tag is excluded, correctly triggering the fallback. If the fallback also finds no tags, the existing message(WARNING "git describe failed") path reports it cleanly.

Verified locally: -- Git describe: v3.5.0-rc5-10-g52f0495 / -- Firebird ODBC Driver version: 3.5.0.11. All 11 CI jobs green on run 24527813786.

irodushka · 2026-04-16T19:25:37Z

Aha. Great)

Another note - let's exclude -DDEBUG from ASAN/Valgrind builds too, this macro adds some extra logging we actually don't need

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 748b82b..0769b1d 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -130,7 +130,7 @@ else()
 
     # Compiler optimization flags (from PR #248 / old makefile.linux)
     add_compile_options(
-        "$<$<CONFIG:Debug>:-O0;-g3;-D_DEBUG;-DDEBUG;-fexceptions>"
+        "$<$<CONFIG:Debug>:-O0;-g3;-D_DEBUG;-fexceptions>"
         "$<$<CONFIG:Release>:-O3;-DNDEBUG;-ftree-loop-vectorize>"
         "$<$<CONFIG:RelWithDebInfo>:-O2;-g;-DNDEBUG>"
         "$<$<CONFIG:MinSizeRel>:-Os;-DNDEBUG>"
@@ -139,6 +139,7 @@ else()
     # LOGGING: Debug builds only; sanitizer builds strip it for cleaner output.
     if(CMAKE_BUILD_TYPE STREQUAL "Debug" AND NOT BUILD_WITH_ASAN AND NOT BUILD_WITH_VALGRIND)
         add_definitions(-DLOGGING)
+        add_definitions(-DDEBUG)
     endif()
 
     # SSE4.1 for x86/x86_64 (from PR #248)

irodushka · 2026-04-16T20:01:43Z

@fdcastel

Bad thing happens.
Your fix

// Before
lengthString = length / sizeof(wchar_t);
// After
lengthString = length / sizeof(SQLWCHAR);

is not good. I get

...
SQLGetDiagRecW
SQLGetDiagRecW
*** stack smashing detected ***: terminated
Aborted (core dumped)

while running ./firebird_odbc_tests, every time, with ASAN, without ASAN, doesn't matter.
Without the fix (lengthString = length / sizeof(wchar_t);) the test suite runs OK but obviously the ASAN build is failing.
I wonder why don't you see this and why we don't see this in the CI logs.

So it's not such a trivial case with that bug.

I will investigate tomorrow for it's a bit late here now. But you understand, I'm holding off on this PR until we solve this mystery.

irodushka · 2026-04-19T18:47:24Z

Please wait. Trying one more thing...

Okay, I'm in no hurry)

Just one thing to say - I looked into PR #283 and, frankly, I don't like what I see in the MainUnicode.cpp and in the reworked class ConvertingString in particular. Overcomplicated, all that redundant(?) logic with multi-bytes (surrogates etc)... Do I understand right you gonna refactor it after our last investigations?

fdcastel · 2026-04-19T18:54:06Z

Okay, I'm in no hurry)

🤣

I looked into PR #283 and, frankly, I don't like what I see in the MainUnicode.cpp and in the reworked class ConvertingString in particular. Overcomplicated, all that redundant(?) logic with multi-bytes (surrogates etc)... Do I understand right you gonna refactor it after our last investigations?

That was just the first attempt 😉 -- no worries!

SURE, We will fix all that. ConvertingString as it is today should be killed with fire.

fdcastel · 2026-04-19T18:56:09Z

Reverted back to length / sizeof(wchar_t) -- the destructor's mbstowcs writeback is now safe again (no more stack smash).

fdcastel · 2026-04-19T18:57:34Z

P.S: Added a minimum-size floor in Alloc().

internal byteString is now at least 8 bytes, so OdbcError::sqlGetDiagRec's strcpy("HY000\0") no longer overflows even when lengthString=3.

This closes the original heap-buffer-overflow without touching the mbstowcs bound.

Follow-up to the ConvertingString discussion in FirebirdSQL#289. The 8-byte floor in Alloc() introduced by the previous commit was a crutch: it avoided the strcpy heap overflow for the SQL-state case but left the destructor's mbstowcs((wchar_t*)unicodeString, ..., lengthString) writing 4-byte wchar_t units into a caller buffer that is SQLWCHAR-sized (2 bytes). Even when that did not overflow, the data was wrong — 'HY000' became 'H' after the client reinterpreted the bytes as UCS-2. Replace the Linux non-connection paths in both directions with the same byte-widening / byte-narrowing loop that unixODBC itself uses internally (ansi_to_unicode_copy / unicode_to_ansi_copy). This is correct for the ASCII-only error/state strings that reach this code; non-ASCII handling remains the subject of the broader rewrite tracked as Tier 9.1 in FirebirdSQL#287. Changes in MainUnicode.cpp: - lengthString now uses sizeof(SQLWCHAR) again (correct SQLWCHAR count). - Destructor Linux branch widens bytes into SQLWCHAR units. - convUnicodeToString Linux branch narrows SQLWCHAR to low bytes. - Temporary NUL is written as a SQLWCHAR-sized zero (not wchar_t). - Remove the Alloc() floor — no longer needed. - Add sqlwcharLen() helper; wcslen() on SQLWCHAR data is unsafe on Linux.

irodushka · 2026-04-20T08:02:28Z

@fdcastel

I really appreciate your work, but I think we're taking a wrong turn.

I remember we decided to separate the ASAN/VALGRING stuff from the unicode fixes. But now you are mixing it all up again.
When I wrote That means - all the ASAN builds will fail, and the driver itself continues to be "toxic"... I meant that it's better to fix the unicode issue asap, not postponing it until some 9.* phase, but a separation is still mandatory. It can be two subsequent PRs, for instance.
When I have to choose between the development speed and code quality, I always (almost always:) choose the latter. You know the saying "Measure twice, cut once"?) We have a Russian very close equivalent, but with "measure seven times", not twice:)

irodushka · 2026-04-20T08:41:49Z

So. What about following the plan written below?)

No need to rush! Very important bullet!
Clean up this PR - leave only ASAN/Valgrind issues. And merge it. ASAN build will fail - ignore it.
Open the separated PR for unicode issues.
Develop a bundle of tests for widechar ODBC calls. These tests must cover the both usage scenarios of ConvertingString class (both of the ctor calls). I suppose that initially all those tests will fail on Linux.
Then I myself will try to implement the correct logic. Having the tests is critical here.
After that you'll merge these changes back into your branch.
Everyone is happy and ~~gay~~ no you cannot say ~~"gay"~~ in that meaning no no no.

fdcastel · 2026-04-20T09:06:36Z

Updated #287: Moved Tasks from Tier 9.1 to Tier 1b.

I’ll be working on the split PRs over the next few hours.

irodushka · 2026-04-20T09:20:19Z

Updated #287: Moved Tasks from Tier 9.1 to Tier 1b.

I’ll be working on the split PRs over the next few hours.

The primary task (after the asan/valgrind PR cleaning) is to develop the widechar odbc tests. Okay?

fdcastel · 2026-04-20T09:23:04Z

The primary task (after the asan/valgrind PR cleaning) is to develop the widechar odbc tests. Okay?

Yep. Loud and clear! 😉

Revert the MainUnicode.cpp changes (832d8e7, 0f8f90f, bea6403) per irodushka's plan in FirebirdSQL#289 comment 4279111183 — the Unicode rewrite is moving to its own PR, preceded by a widechar-tests PR. Restore continue-on-error on the ASAN matrix job temporarily; Valgrind stays strict. ASAN will flag the heap-buffer-overflow in SQLGetDiagRecW again; this is the known issue the follow-up PRs will fix and is tracked as issue FirebirdSQL#287 Tier 1b.

fdcastel · 2026-04-20T16:11:04Z

@irodushka — split done per your plan:

Add ASAN and Valgrind integration for CI test suite #289 (this PR) trimmed to ASAN/Valgrind CI only. MainUnicode.cpp reverted to master. ASAN job is continue-on-error while the Unicode fix is pending (Valgrind stays strict). Ready to merge.
Add widechar ODBC call tests (driving Unicode fix) #290 — widechar ODBC call tests covering both ConvertingString ctor paths (output: SQLGetDiagRecW/SQLErrorW; input: SQLExecDirectW/SQLPrepareW). Linux runs skip with a pointer back here; Windows runs them for real.
Fix SQLWCHAR conversion paths on Linux (draft) #291 — draft reference implementation of the Linux Unicode fix against Add widechar ODBC call tests (driving Unicode fix) #290's tests. Yours to approve, rewrite on top of, or close-and-replace — whichever fits step 5 of your plan.

Suggested order: merge #289 → review #290 → take over #291.

fdcastel · 2026-04-20T16:19:10Z

Just a reminder: these are items that were intentionally left out of scope for this plan/PR.

The connection->MbsToWcs / WcsToMbs Linux branch in MainUnicode.cpp (around lines 129 and 195) is still incorrect in the same way, but it is not exercised by the current test surface.
True iconv / UTF-8 correctness for non-ASCII input.
Redesign of ConvertingString (destructor-as-converter, silent error handling, etc.).

All of these belong in #287 (Tier 1b). After #291 has landed.

fdcastel · 2026-04-20T16:33:25Z

P.S.: As expected, ASAN is failing now. It will be green after #291, too. 😉

irodushka · 2026-04-20T17:09:51Z

I think we should remove the Draft tag, disable (temporarily) the ASAN build&test and merge this PR to the master branch.
Are you ok with it?

…ectW / SQLPrepareW) Exercises both ConvertingString constructor paths in MainUnicode.cpp: output buffer (SQLGetDiagRecW / SQLErrorW) and input buffer (SQLExecDirectW / SQLPrepareW). Linux tests skip with a pointer to the follow-up Unicode rewrite PR; they are meant to drive that PR's implementation. Windows runs them for real. Per the plan in #289 comment 4279111183. Tracked as issue #287 Tier 1b.

@irodushka

LPWSTR is WCHAR* = unsigned short* = SQLWCHAR* on both Windows and unixODBC/Linux, so `*(LPWSTR)ptr = L'\0'` writes 2 bytes via narrowing store, not 4 — no overrun. Thanks to @irodushka for the correction (FirebirdSQL#289 discussion_r3109004999). Keeping the replacement `unicodeString[len] = 0;` which he endorsed as more compact and understandable. The sibling comment in convUnicodeToString stays — that one is a real 4-byte write through an explicit `(wchar_t*)` cast.

@irodushka

Per FirebirdSQL#289 discussion with @irodushka: rather than keep ASAN running under continue-on-error, disable it outright until the Unicode rewrite lands (draft PR FirebirdSQL#291, tracked as FirebirdSQL#287 Tier 1b). Re-enable the matrix entry once FirebirdSQL#291 merges. Valgrind remains strict.

fdcastel · 2026-04-20T17:59:12Z

Agreed. Done in 98115a1:

ASAN matrix entry commented out (with a pointer back to Fix SQLWCHAR conversion paths on Linux (draft) #291 / [EPIC] vNext Migration Plan #287 Tier 1b for re-enabling).
continue-on-error hack removed — no longer needed.
Valgrind stays strict.
PR marked ready for review.

Leaving the merge to you 😉.

fdcastel · 2026-04-20T18:12:02Z

I need a beer! 🤣

P.S; @irodushka Thank you very much for your patience and guidance. Much appreciated! 🤗

irodushka · 2026-04-20T18:20:26Z

I need a beer! 🤣

Come to Moscow))

fdcastel · 2026-04-20T18:23:35Z

Come to Moscow))

Second invite in less than a month. Adding it to my ever-growing “dangerously tempting ideas” list. 😅

fdcastel marked this pull request as draft April 10, 2026 23:45

fdcastel marked this pull request as ready for review April 11, 2026 00:09

fdcastel mentioned this pull request Apr 11, 2026

[EPIC] vNext Migration Plan #287

Open

fdcastel added 5 commits April 11, 2026 17:24

Remove continue-on-error from ASAN CI job

8ea653f

The heap-buffer-overflow in ConvertingString (sizeof(wchar_t) vs sizeof(SQLWCHAR)) has been fixed. ASAN now passes all 375 tests cleanly on ubuntu-22.04/GCC, so the soft-fail guard is no longer needed.

fdcastel force-pushed the fix-issue-288 branch from d1a651d to 8ea653f Compare April 11, 2026 20:25

irodushka reviewed Apr 15, 2026

View reviewed changes

Comment thread CMakeLists.txt Outdated

irodushka reviewed Apr 15, 2026

View reviewed changes

Comment thread CMakeLists.txt Outdated

irodushka reviewed Apr 15, 2026

View reviewed changes

Comment thread CMakeLists.txt

irodushka reviewed Apr 15, 2026

View reviewed changes

Comment thread CMakeLists.txt Outdated

fdcastel added 2 commits April 15, 2026 16:22

irodushka reviewed Apr 16, 2026

View reviewed changes

Comment thread CMakeLists.txt Outdated

Simplify LOGGING guard to a single combined condition

52f0495

Per review feedback: remove_definitions() cannot undo definitions added via add_compile_options(), so the add-then-remove pattern was misleading. Collapse to a single if() with the combined condition instead.

fdcastel marked this pull request as ready for review April 19, 2026 19:00

irodushka reviewed Apr 19, 2026

View reviewed changes

Comment thread MainUnicode.cpp Outdated

irodushka requested changes Apr 20, 2026

View reviewed changes

Comment thread MainUnicode.cpp Outdated

This was referenced Apr 20, 2026

Add widechar ODBC call tests (driving Unicode fix) #290

Merged

Fix SQLWCHAR conversion paths on Linux (draft) #291

Draft

fdcastel marked this pull request as draft April 20, 2026 16:15

fdcastel marked this pull request as ready for review April 20, 2026 17:59

irodushka merged commit ca97f46 into FirebirdSQL:master Apr 20, 2026
10 checks passed

Conversation

fdcastel commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

CMake (CMakeLists.txt)

CMake Presets (CMakePresets.json)

Build script (firebird-odbc-driver.build.ps1)

CI (build-and-test.yml)

Valgrind suppressions (valgrind.supp)

Local usage

Uh oh!

fdcastel commented Apr 10, 2026

Uh oh!

fdcastel commented Apr 11, 2026

Bug found and fixed by ASAN

What ASAN reported

Root cause

Fix

CI after fix

Uh oh!

fdcastel commented Apr 11, 2026

Uh oh!

irodushka commented Apr 11, 2026

Uh oh!

fdcastel commented Apr 11, 2026

Uh oh!

fdcastel commented Apr 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fdcastel commented Apr 15, 2026

Uh oh!

Uh oh!

irodushka commented Apr 16, 2026

Uh oh!

fdcastel commented Apr 16, 2026

Uh oh!

irodushka commented Apr 16, 2026

Uh oh!

irodushka commented Apr 16, 2026

Uh oh!

irodushka commented Apr 19, 2026

Uh oh!

fdcastel commented Apr 19, 2026

Uh oh!

fdcastel commented Apr 19, 2026

Uh oh!

fdcastel commented Apr 19, 2026

Uh oh!

Uh oh!

Uh oh!

irodushka commented Apr 20, 2026

Uh oh!

irodushka commented Apr 20, 2026

Uh oh!

fdcastel commented Apr 20, 2026

Uh oh!

irodushka commented Apr 20, 2026

Uh oh!

fdcastel commented Apr 20, 2026

Uh oh!

fdcastel commented Apr 20, 2026

Uh oh!

fdcastel commented Apr 20, 2026

Uh oh!

fdcastel commented Apr 20, 2026

Uh oh!

irodushka commented Apr 20, 2026

Uh oh!

fdcastel commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

fdcastel commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fdcastel commented Apr 10, 2026 •

edited

Loading

CMake (`CMakeLists.txt`)

CMake Presets (`CMakePresets.json`)

Build script (`firebird-odbc-driver.build.ps1`)

CI (`build-and-test.yml`)

Valgrind suppressions (`valgrind.supp`)

fdcastel commented Apr 11, 2026 •

edited

Loading

fdcastel commented Apr 20, 2026 •

edited

Loading

fdcastel commented Apr 20, 2026 •

edited

Loading